Skip to content

fix(contracts): state the scroll keyboard clip once for every platform - #2537

Merged
thymikee merged 3 commits into
mainfrom
feat/scroll-keyboard-contract-2500
Sep 13, 2026
Merged

thymikee merged 3 commits into
mainfrom
feat/scroll-keyboard-contract-2500

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

First layer of #2500: the rule that keeps a scroll's swipe out of the keyboard, stated once in TypeScript, with the golden table both native runners read. iOS implements it in #2503 and Android in #2514, so nothing here moves a gesture yet.

clipScrollViewportAboveKeyboard is the whole decision: a keyboard that reaches the swipe's centre line takes the band below it, and when what remains cannot carry a swipe the command refuses with scrollKeyboardOccludesSurfaceError rather than scrolling a surface the keys own. contracts/fixtures/scroll-keyboard-policy.json is the cross-language table, so the Swift and Java lanes assert against the same rows instead of each platform inventing its own thresholds.

Behaviour stated here

  • A keyboard is struck when it reaches the swipe's centre line, and a floating keyboard sitting past that line is not — its width fraction no longer decides anything.
  • keyboardAvoided and keyboardMinY join a scroll response only when the clip ran, and the CLI help, MCP output schema and command docs name them here, so both platforms answer with one shape.
  • The keyboard is never dismissed. Dismissing drops focus and breaks a fill/scroll/fill loop; keyboard dismiss stays an explicit command.
  • The refusal code lands before any runner emits it, which is deliberate: the typed reason and its evidence keys are defined once here, and fix(scroll): keep the swipe above the keyboard, refuse when it cannot #2503 and fix(android): keep a scroll's swipe out of the IME window #2514 are what produce them.

Validation

pnpm check green at 1b8548af6d, which covers the golden parity table (18 rows, including the Android-shaped adjustPan window with only 88px of 900px left above the IME is refused and the iOS-shaped landscape keyboard leaving 28pt of a 402pt axis is refused instead of swiping under the keys), the Apple-side mapping tests, MCP metadata and the production-exports lane.

No device run belongs to this layer — it dispatches nothing. The live simulator and emulator evidence for the paths it states is posted on #2503 and #2514.

Size

11 files, 615 gross lines: one shared module, its golden table, and the surface that names its two response keys.

Part of the #2500 stack: this PR -> #2503 (iOS runner) -> #2514 (Android). Do not merge out of order.

@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.50 MB 4.50 MB +1.8 kB
Package (unpacked) 4.50 MB 4.50 MB +1.8 kB
Package (download) 1.33 MB 1.33 MB +748 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 20.5 ms 21.3 ms +0.7 ms
CLI --help 57.7 ms 59.4 ms +1.7 ms

@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-13 11:55 UTC

@thymikee

Copy link
Copy Markdown
Member Author

No actionable findings on 1b8548a. The shared contract and Apple surface extraction preserves the previously reviewed source, with the dispatch changes correctly left in the dependent PRs. Typed errors, response fields, MCP schema, fixtures, help, and docs are aligned for this layer. Current checks pass and there are no conflicts. Ready for human review and merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 12, 2026
@thymikee
thymikee added this pull request to stack #2539 September 13, 2026 05:56
…nd keep the runner's message

The Apple scroll owner rebuilt the refusal per command, discarding the
runner's measured message and carrying an unmeasured variant of the
error builder for it. The shared reason and hint are now one frozen
object in scroll-gesture; the Apple owner adds it to the runner's own
error (matched on the typed runner code, transport details kept), and
the error builder takes a plain measured occlusion, which only Android
produces in-process. The help text names the behaviour in one clause;
the hint carries the recovery at the moment it matters.
@thymikee

Copy link
Copy Markdown
Member Author

Reviewed the whole stack and pushed a simplification at 673ea455d6 (layers 2 and 3 rebased on top, content of #2503 unchanged).

What changed here

  • scrollKeyboardOccludesSurfaceError takes a plain measured occlusion (keyboardMinY, visibleHeight, viewportHeight), which only Android produces in-process. The unmeasured variant, the Extract<…> & {…} type and the test that pinned the fabricated message are gone.
  • The reason and hint are one frozen SCROLL_KEYBOARD_OCCLUDES_SURFACE_DETAILS in scroll-gesture. The Apple owner now adds it to the runner's own error and keeps the runner's measured message and transport details, instead of rebuilding the error with a message that had no number in it.
  • The scroll help text is one short clause; the error hint carries the recovery when it happens.

Net −41 lines on this layer, same reason and hint on every path.

Tried and reverted: hosting the details in classifyRunnerReportedError so the lifecycle-journal recovery route gets them for free. Any value import from contracts into runner-contract.ts (or scroll-gesture → scroll-command) grows the eager closures and fails eager-closure-budgets, so the per-command wrapper stays. The journal route is still covered because the wrapper matches on details.runnerErrorCode, which recovery sets.

pnpm check green on the stack head cef825eb77.

@thymikee

Copy link
Copy Markdown
Member Author

No actionable findings on 673ea45. The Apple wrapper preserves the runner message and transport details while adding the shared reason and hint, including after journal recovery.

Coverage failed in the unchanged snapshot-source lifecycle fixture: "a crashed helper is removed and the next request starts a fresh helper" exceeded its dispatch deadline and raised bridge-connect-deadline. This looks unrelated to the scroll delta, but CI needs to pass before merge. The ready-for-human label remains valid. Failure: https://github.com/callstack/agent-device/actions/runs/34748827127/job/103701514870

@thymikee
thymikee merged commit 5cf6414 into main Sep 13, 2026
14 of 15 checks passed
@thymikee
thymikee deleted the feat/scroll-keyboard-contract-2500 branch September 13, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant